Binary Reading: Avoid overlapping internal names between imports and non-imports #7700
Conversation
src/wasm/wasm-binary.cpp
Outdated
| // We found a name in the names section. Use it, and also note it as used | ||
| // so we don't generate such a name below, later. |
There was a problem hiding this comment.
Is there anything stopping us from generating a name below and later running into it in the names section?
There was a problem hiding this comment.
I believe this code aims to avoid that, be pre-populating the names:
binaryen/src/wasm/wasm-binary.cpp
Lines 3010 to 3020 in efcf2dc
But that raises the question of why that didn't fix this issue. Unfortunately, I can only reproduce this with an experimental pass not yet in a PR, so I don't have a good testcase. Anyhow, let's hold off on this until that PR is ready.
There was a problem hiding this comment.
Ok, the issue is that this code is for non-imports, and that imports are handled separately... that is, the bug is that we avoided name overlaps in two different places, imports and non-imports, so we didn't handle overlaps between them. I pushed a fix now that makes us use a single namespace for this deduplication.
kripken
changed the title
(updated description)
We handled name overlaps (name section name that happens to collide
with the next internal name we invent for a non-named thing), but we
did it separately for imports and non-imports, allowing a collision
between them in very odd situations. To fix that, maintain a single shared
list of used names for imports and non-imports.